fix(certbot): reissue when the configured domains change - #1137
Merged
Conversation
…keys
`certbot cfg` walks the serialized document and the struct's doc comments in
lockstep, by position. An `Option` that is `None` serializes to nothing, so
`cf_api_url` and `renewed_hook` are absent from the document and every comment
after the first of them describes the key above the one it belongs to:
# Optional Cloudflare-compatible API base URL
dns_txt_ttl = 60
# TTL for DNS TXT challenge records in seconds
auto_set_caa = true
Look the comment up by key name instead. A key with no doc comment keeps none
rather than borrowing its neighbour's.
The CLI keeps the ACME account key and the certificate key in a plain directory on the host that runs it, so nothing about the issuance is attested. A deployed gateway issues its own certificates from the configuration it already holds and publishes the public keys for `ct_monitor` to check. Say so in a README, next to what the subcommands do and where the workdir keeps things. `init`'s help said it initializes the configuration file; it reads the configuration file and creates the ACME account.
kvinwang
force-pushed
the
fix/certbot-san-config
branch
from
August 26, 2026 03:23
1431ee1 to
a896a25
Compare
Step 4 told the operator to configure `GATEWAY_CERT`/`GATEWAY_KEY` in `build-config.sh` and run `./certbot renew -c certbot.toml` on the host. That is not how a deployed gateway gets a certificate, and `build-config.sh` is not part of this guide's flow: `dstack-gateway` links the `certbot` crate and runs it inside the CVM, answering dns-01 with the DNS credential in its admin config and keeping the ACME account key and every certificate in the CVM's WaveKV store. Following the old text produced a certificate on the host that nothing reads. Describe what step 3 already set up -- `bootstrap-cluster.sh` calling SetCertbotConfig, CreateDnsCredential and AddZtDomain -- how to watch issuance land in `ListZtDomains`, how to pin CAA, and what switching from staging to production takes. Zero-trust HTTPS moves to the required part of the checklist: without a certificate for the domain the gateway cannot serve an app over TLS.
The production setup guide walked the operator through editing `certbot.toml`, running `./certbot set-caa && ./certbot renew` by hand, and pointing `cert_chain`/`cert_key` at the result. A gateway running on the host links the same `certbot` crate as one running in a CVM and issues over dns-01 from its own process, keeping the ACME account key and every certificate in its WaveKV store -- `cert_chain`/`cert_key` only load a certificate something else produced. Configure the gateway instead: the proxy fields, the admin API the ACME settings are stored through, and a `data_dir` that survives restarts so a restart does not ask the CA for a fresh certificate. Then SetCertbotConfig, CreateDnsCredential and AddZtDomain, with what to watch in ListZtDomains, what SetCaa pins, and what switching from staging to production takes. Also: `core.admin.auth_token` is the current key name (`admin_token` is the accepted older one), URL Format becomes its own section rather than sitting inside a configuration step, and the alerting table listed `dstack_gateway_kv_persist_failures_total` twice.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Editing
domainsincertbot.tomldoes nothing once a certificate exists. Theoperator adds a name, restarts certbot, and every run reports the certificate as
current:
Two places have to agree on the name list, and neither consults the
configuration:
So the name list is pinned at whatever the first issuance used, and renewal
copies it forward. The configuration edit is not deferred until expiry -- it
never takes effect at all, and nothing in the log says so.
This is the CLI's issuance path (
certbot/src/bot.rs, used bycertbot/cli), which is how self-hosted deployments obtain the gatewaycertificate --
docs/deployment.mdanddocs/dstack-gateway.mdboth drive itthrough
certbot.toml. The in-CVM gateway path is unaffected:DistributedCertBotcomputes
*.{domain}from its config on every issuance and never callsrenew_cert.Fix
The live certificate is only the certificate that was asked for if it carries
the configured names, so
create_cert_if_neededcompares them and reissues on amismatch:
names_matchcompares sets, not sequences: the CA returns names in its ownorder (the staging CA puts the wildcard first), DNS names are case-insensitive,
and a trailing root dot is the same name. Equality rather than containment, so
narrowing the list reissues too -- a certificate covering more than the
configuration asks for is not the configured certificate either.
An unreadable live certificate reissues rather than being left alone: it cannot
be checked against the configuration, so continuing to serve it means serving
something this process can no longer reason about. Issuance replaces it.
renew_certstill derives its names from the certificate it replaces, which iscorrect once the certificate itself is kept in step:
create_cert_if_neededruns first on every cycle, so renewal only ever sees an aligned certificate.
The existing key is reused, as renewal already does.
A reissue that fails must not take the renewal down with it. The configuration
can name something the CA will never validate -- a typo, a zone the Cloudflare
token cannot write -- and that reissue then fails on every cycle. Failing the
run at that point would skip the renewal check below it, so one name the
operator got wrong would stop renewing the certificate that is actually being
served, until it expires 90 days later: a worse outcome than the edit being
ignored, which is what this PR started from.
renew_innerreports the failureand carries on to the renewal, and returns the failure only if the renewal
committed nothing of its own -- so a daemon keeps the deployment alive, a
renew --oncestill exits non-zero, and a renewal that did commit still runsthe
renewed_hook. Nothing to protect on a first run, where there is no livecertificate: the error propagates as before.
The decision itself is
reissue_reason(live_cert_pem, domains) -> Option<String>,a pure function of the certificate and the configuration, so all three of its
outcomes are unit-testable. A live certificate that cannot even be read off disk
takes the same path as one that cannot be parsed, rather than failing the run.
Verification
Pebble + a mock Cloudflare API. Same workdir,
domainsedited between the tworuns,
origin/next(55021edbf8) and this branch:origin/next["sanchange.e2e.test"]DNS:sanchange.e2e.testDNS:sanchange.e2e.test["sanchange.e2e.test", "extra.e2e.test"]certificate ... is up to date, no new certificateDNS:sanchange.e2e.test, DNS:extra.e2e.testorigin/nextends with one certificate in the backup directory, this branchwith two. The reissue announces itself:
Two distinct names rather than a name plus its wildcard, deliberately: a
base-plus-wildcard order fails on
nextfor an unrelated reason, fixedseparately in #1136.
Unit tests cover
names_match(reordering and case are not changes; an added ordropped name is) and
reissue_reasonagainst real self-signed certificates (theconfigured one is kept, a missing name reissues and names both lists, an
unparseable certificate reissues).
cargo fmt,cargo clippy -p certbot -p certbot-cli --all-targets -D warningsandcargo test -p certbotare clean.Two adjacent fixes
Found while documenting the CLI, both in separate commits so they can be dropped
independently.
certbot cfgwalked the serialized document and the struct's doc comments inlockstep by position, and a
Noneoption serializes to nothing -- so withcf_api_urlabsent, every comment from there on described the key above the oneit belonged to:
The comment is now looked up by key name.
init's help said it initializes theconfiguration file; it reads the configuration file and creates the ACME
account.
The CLI also gets a README saying what it is for: it keeps the ACME account key
and the certificate key in a plain directory on the host that runs it, so
nothing about the issuance is attested, while a deployed gateway issues its own
certificates from the configuration it already holds and publishes the public
keys for
ct_monitorto check.The deployment guide was pointing at the CLI
docs/deployment.mdstep 4 told the operator to setGATEWAY_CERT/GATEWAY_KEYin
build-config.shand run./certbot renew -c certbot.tomlon the host. Agateway deployed by that guide runs as a CVM and issues its own certificates
from its admin config, keeping the ACME account key and every certificate in the
CVM's WaveKV store -- so the old text produced a certificate on the host that
nothing reads, from a file that is not part of the guide's flow.
The step now describes what step 3's
bootstrap-cluster.shalready configured(
SetCertbotConfig,CreateDnsCredential,AddZtDomain), how to watchissuance land in
ListZtDomains, howSetCaapins issuance to the gateway'sown ACME account, and what switching from staging to production takes. It moves
to the required part of the checklist: without a certificate for the domain the
gateway cannot serve an app over TLS.
docs/dstack-gateway.mdhad the same problem in the other deployment shape: itwalked a host-mode gateway through
certbot.toml,./certbot set-caa && ./certbot renew, andcert_chain/cert_key. A gateway on the host links the same crateand issues from its own process just as one in a CVM does, so that guide now
configures the gateway -- the proxy fields, the admin API the ACME settings are
stored through, and a
data_dirthat survives restarts -- and then walks thesame three admin RPCs.
cert_chain/cert_keyare documented for what they are:a certificate something else produced, loaded at startup.
Two things noticed in that file while editing it:
core.admin.auth_tokenis thecurrent key name (the guide used the accepted older
admin_token), and thealerting table listed
dstack_gateway_kv_persist_failures_totaltwice.